Skip to content

Conversation

@vdonaldson
Copy link
Contributor

Intrinsic module procedures ieee_get_modes, ieee_set_modes, ieee_get_status, and ieee_set_status store and retrieve opaque data values whose size varies by machine and OS environment. These data values are usually, but not always small. Their sizes are not directly known in a cross compilation environment. Address this issue by implementing two mechanisms for processing these data values. Environments that use typical small data sizes can access storage defined at compile time. When this is not valid, data storage of any size can be allocated at runtime.

@llvmbot llvmbot added flang:runtime flang Flang issues not falling into any other category flang:fir-hlfir flang:semantics labels Jan 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 7, 2025

@llvm/pr-subscribers-flang-fir-hlfir
@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-runtime

Author: None (vdonaldson)

Changes

Intrinsic module procedures ieee_get_modes, ieee_set_modes, ieee_get_status, and ieee_set_status store and retrieve opaque data values whose size varies by machine and OS environment. These data values are usually, but not always small. Their sizes are not directly known in a cross compilation environment. Address this issue by implementing two mechanisms for processing these data values. Environments that use typical small data sizes can access storage defined at compile time. When this is not valid, data storage of any size can be allocated at runtime.


Patch is 51.50 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/121949.diff

13 Files Affected:

  • (modified) flang/include/flang/Evaluate/target.h (+4)
  • (modified) flang/include/flang/Optimizer/Builder/IntrinsicCall.h (+2-4)
  • (modified) flang/include/flang/Optimizer/Builder/Runtime/Exceptions.h (+4)
  • (modified) flang/include/flang/Runtime/exceptions.h (+4)
  • (modified) flang/include/flang/Runtime/magic-numbers.h (+4-5)
  • (modified) flang/include/flang/Tools/TargetSetup.h (+3)
  • (modified) flang/lib/Evaluate/target.cpp (+1)
  • (modified) flang/lib/Optimizer/Builder/IntrinsicCall.cpp (+55-25)
  • (modified) flang/lib/Optimizer/Builder/Runtime/Exceptions.cpp (+14)
  • (modified) flang/module/__fortran_ieee_exceptions.f90 (+4-2)
  • (modified) flang/runtime/exceptions.cpp (+12-10)
  • (modified) flang/test/Lower/Intrinsics/ieee_femodes.f90 (+39-32)
  • (modified) flang/test/Lower/Intrinsics/ieee_festatus.f90 (+81-74)
diff --git a/flang/include/flang/Evaluate/target.h b/flang/include/flang/Evaluate/target.h
index 154561ce868eb1..e07f916b875e06 100644
--- a/flang/include/flang/Evaluate/target.h
+++ b/flang/include/flang/Evaluate/target.h
@@ -112,6 +112,9 @@ class TargetCharacteristics {
   bool isPPC() const { return isPPC_; }
   void set_isPPC(bool isPPC = false);
 
+  bool isSPARC() const { return isSPARC_; }
+  void set_isSPARC(bool isSPARC = false);
+
   bool isOSWindows() const { return isOSWindows_; }
   void set_isOSWindows(bool isOSWindows = false) {
     isOSWindows_ = isOSWindows;
@@ -126,6 +129,7 @@ class TargetCharacteristics {
   std::uint8_t align_[common::TypeCategory_enumSize][maxKind + 1]{};
   bool isBigEndian_{false};
   bool isPPC_{false};
+  bool isSPARC_{false};
   bool isOSWindows_{false};
   bool haltingSupportIsUnknownAtCompileTime_{false};
   bool areSubnormalsFlushedToZero_{false};
diff --git a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
index 3d0516555f761b..c458aa89fc956a 100644
--- a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
+++ b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
@@ -268,10 +268,8 @@ struct IntrinsicLibrary {
   mlir::Value genIeeeCopySign(mlir::Type, llvm::ArrayRef<mlir::Value>);
   void genIeeeGetFlag(llvm::ArrayRef<fir::ExtendedValue>);
   void genIeeeGetHaltingMode(llvm::ArrayRef<fir::ExtendedValue>);
-  template <bool isGet>
-  void genIeeeGetOrSetModes(llvm::ArrayRef<fir::ExtendedValue>);
-  template <bool isGet>
-  void genIeeeGetOrSetStatus(llvm::ArrayRef<fir::ExtendedValue>);
+  template <bool isGet, bool isModes>
+  void genIeeeGetOrSetModesOrStatus(llvm::ArrayRef<fir::ExtendedValue>);
   void genIeeeGetRoundingMode(llvm::ArrayRef<fir::ExtendedValue>);
   void genIeeeGetUnderflowMode(llvm::ArrayRef<fir::ExtendedValue>);
   mlir::Value genIeeeInt(mlir::Type, llvm::ArrayRef<mlir::Value>);
diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Exceptions.h b/flang/include/flang/Optimizer/Builder/Runtime/Exceptions.h
index f44e0c95ef6d4a..7487444f3a7a95 100644
--- a/flang/include/flang/Optimizer/Builder/Runtime/Exceptions.h
+++ b/flang/include/flang/Optimizer/Builder/Runtime/Exceptions.h
@@ -33,5 +33,9 @@ mlir::Value genGetUnderflowMode(fir::FirOpBuilder &builder, mlir::Location loc);
 void genSetUnderflowMode(fir::FirOpBuilder &builder, mlir::Location loc,
                          mlir::Value bit);
 
+mlir::Value genGetModesTypeSize(fir::FirOpBuilder &builder, mlir::Location loc);
+mlir::Value genGetStatusTypeSize(fir::FirOpBuilder &builder,
+                                 mlir::Location loc);
+
 } // namespace fir::runtime
 #endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_EXCEPTIONS_H
diff --git a/flang/include/flang/Runtime/exceptions.h b/flang/include/flang/Runtime/exceptions.h
index 483d0271bcab00..dddcc7670f6637 100644
--- a/flang/include/flang/Runtime/exceptions.h
+++ b/flang/include/flang/Runtime/exceptions.h
@@ -32,6 +32,10 @@ bool RTNAME(SupportHalting)(uint32_t except);
 bool RTNAME(GetUnderflowMode)(void);
 void RTNAME(SetUnderflowMode)(bool flag);
 
+// Get the byte size of ieee_modes_type and ieee_status_type data.
+std::size_t RTNAME(GetModesTypeSize)(void);
+std::size_t RTNAME(GetStatusTypeSize)(void);
+
 } // extern "C"
 } // namespace Fortran::runtime
 #endif // FORTRAN_RUNTIME_EXCEPTIONS_H_
diff --git a/flang/include/flang/Runtime/magic-numbers.h b/flang/include/flang/Runtime/magic-numbers.h
index 1d3c5dca0b4bfb..6788ba098bcf99 100644
--- a/flang/include/flang/Runtime/magic-numbers.h
+++ b/flang/include/flang/Runtime/magic-numbers.h
@@ -118,11 +118,10 @@ ieee_arithmetic module rounding procedures.
 #define _FORTRAN_RUNTIME_IEEE_OTHER 5
 
 #if 0
-The size of derived types ieee_modes_type and ieee_status_type from intrinsic
-module ieee_exceptions must be large enough to hold an fenv.h object of type
-femode_t and fenv_t, respectively. These types have members that are declared
-as int arrays with the following extents to allow build time validation of
-these sizes in cross compilation environments.
+INTEGER(kind=4) extents for ieee_exceptions module types ieee_modes_type and
+ieee_status_type. These extent values are large enough to hold femode_t and
+fenv_t data in many environments. An environment that does not meet these
+size constraints may allocate memory with runtime size values.
 #endif
 #define _FORTRAN_RUNTIME_IEEE_FEMODE_T_EXTENT 2
 #define _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT 8
diff --git a/flang/include/flang/Tools/TargetSetup.h b/flang/include/flang/Tools/TargetSetup.h
index 709c4bbe4b7b0b..d1b0da3a42c897 100644
--- a/flang/include/flang/Tools/TargetSetup.h
+++ b/flang/include/flang/Tools/TargetSetup.h
@@ -71,6 +71,9 @@ namespace Fortran::tools {
   if (targetTriple.isPPC())
     targetCharacteristics.set_isPPC(true);
 
+  if (targetTriple.isSPARC())
+    targetCharacteristics.set_isSPARC(true);
+
   if (targetTriple.isOSWindows())
     targetCharacteristics.set_isOSWindows(true);
 
diff --git a/flang/lib/Evaluate/target.cpp b/flang/lib/Evaluate/target.cpp
index 409e28c767e1e1..94dc35ecd5900a 100644
--- a/flang/lib/Evaluate/target.cpp
+++ b/flang/lib/Evaluate/target.cpp
@@ -104,6 +104,7 @@ void TargetCharacteristics::set_isBigEndian(bool isBig) {
 }
 
 void TargetCharacteristics::set_isPPC(bool isPowerPC) { isPPC_ = isPowerPC; }
+void TargetCharacteristics::set_isSPARC(bool isSPARC) { isSPARC_ = isSPARC; }
 
 void TargetCharacteristics::set_areSubnormalsFlushedToZero(bool yes) {
   areSubnormalsFlushedToZero_ = yes;
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index 9a3777994a9df0..087db38ebebfcd 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -317,13 +317,15 @@ static constexpr IntrinsicHandler handlers[]{
     {"ieee_get_halting_mode",
      &I::genIeeeGetHaltingMode,
      {{{"flag", asValue}, {"halting", asAddr}}}},
-    {"ieee_get_modes", &I::genIeeeGetOrSetModes</*isGet=*/true>},
+    {"ieee_get_modes",
+     &I::genIeeeGetOrSetModesOrStatus</*isGet=*/true, /*isModes=*/true>},
     {"ieee_get_rounding_mode",
      &I::genIeeeGetRoundingMode,
      {{{"round_value", asAddr, handleDynamicOptional},
        {"radix", asValue, handleDynamicOptional}}},
      /*isElemental=*/false},
-    {"ieee_get_status", &I::genIeeeGetOrSetStatus</*isGet=*/true>},
+    {"ieee_get_status",
+     &I::genIeeeGetOrSetModesOrStatus</*isGet=*/true, /*isModes=*/false>},
     {"ieee_get_underflow_mode",
      &I::genIeeeGetUnderflowMode,
      {{{"gradual", asAddr}}},
@@ -367,13 +369,15 @@ static constexpr IntrinsicHandler handlers[]{
     {"ieee_set_flag", &I::genIeeeSetFlagOrHaltingMode</*isFlag=*/true>},
     {"ieee_set_halting_mode",
      &I::genIeeeSetFlagOrHaltingMode</*isFlag=*/false>},
-    {"ieee_set_modes", &I::genIeeeGetOrSetModes</*isGet=*/false>},
+    {"ieee_set_modes",
+     &I::genIeeeGetOrSetModesOrStatus</*isGet=*/false, /*isModes=*/true>},
     {"ieee_set_rounding_mode",
      &I::genIeeeSetRoundingMode,
      {{{"round_value", asValue, handleDynamicOptional},
        {"radix", asValue, handleDynamicOptional}}},
      /*isElemental=*/false},
-    {"ieee_set_status", &I::genIeeeGetOrSetStatus</*isGet=*/false>},
+    {"ieee_set_status",
+     &I::genIeeeGetOrSetModesOrStatus</*isGet=*/false, /*isModes=*/false>},
     {"ieee_set_underflow_mode", &I::genIeeeSetUnderflowMode},
     {"ieee_signaling_eq",
      &I::genIeeeSignalingCompare<mlir::arith::CmpFPredicate::OEQ>},
@@ -4094,11 +4098,12 @@ void IntrinsicLibrary::genRaiseExcept(int excepts, mlir::Value cond) {
 // Return a reference to the contents of a derived type with one field.
 // Also return the field type.
 static std::pair<mlir::Value, mlir::Type>
-getFieldRef(fir::FirOpBuilder &builder, mlir::Location loc, mlir::Value rec) {
+getFieldRef(fir::FirOpBuilder &builder, mlir::Location loc, mlir::Value rec,
+            unsigned index = 0) {
   auto recType =
       mlir::dyn_cast<fir::RecordType>(fir::unwrapPassByRefType(rec.getType()));
-  assert(recType.getTypeList().size() == 1 && "expected exactly one component");
-  auto [fieldName, fieldTy] = recType.getTypeList().front();
+  assert(index < recType.getTypeList().size() && "not enough components");
+  auto [fieldName, fieldTy] = recType.getTypeList()[index];
   mlir::Value field = builder.create<fir::FieldIndexOp>(
       loc, fir::FieldType::get(recType.getContext()), fieldName, recType,
       fir::getTypeParams(rec));
@@ -4488,15 +4493,52 @@ void IntrinsicLibrary::genIeeeGetHaltingMode(
 }
 
 // IEEE_GET_MODES, IEEE_SET_MODES
-template <bool isGet>
-void IntrinsicLibrary::genIeeeGetOrSetModes(
+// IEEE_GET_STATUS, IEEE_SET_STATUS
+template <bool isGet, bool isModes>
+void IntrinsicLibrary::genIeeeGetOrSetModesOrStatus(
     llvm::ArrayRef<fir::ExtendedValue> args) {
   assert(args.size() == 1);
-  mlir::Type ptrTy = builder.getRefType(builder.getIntegerType(32));
   mlir::Type i32Ty = builder.getIntegerType(32);
-  mlir::Value addr =
-      builder.create<fir::ConvertOp>(loc, ptrTy, getBase(args[0]));
-  genRuntimeCall(isGet ? "fegetmode" : "fesetmode", i32Ty, addr);
+  mlir::Type i64Ty = builder.getIntegerType(64);
+  mlir::Type ptrTy = builder.getRefType(i32Ty);
+  mlir::Value addr;
+  if (fir::getTargetTriple(builder.getModule()).isSPARC()) {
+    // Floating point environment data is larger than the __data field
+    // allotment. Allocate data space from the heap.
+    auto [fieldRef, fieldTy] =
+        getFieldRef(builder, loc, fir::getBase(args[0]), 1);
+    addr = builder.create<fir::BoxAddrOp>(
+        loc, builder.create<fir::LoadOp>(loc, fieldRef));
+    mlir::Type heapTy = addr.getType();
+    mlir::Value allocated = builder.create<mlir::arith::CmpIOp>(
+        loc, mlir::arith::CmpIPredicate::ne,
+        builder.createConvert(loc, i64Ty, addr),
+        builder.createIntegerConstant(loc, i64Ty, 0));
+    auto ifOp = builder.create<fir::IfOp>(loc, heapTy, allocated,
+                                          /*withElseRegion=*/true);
+    builder.setInsertionPointToStart(&ifOp.getThenRegion().front());
+    builder.create<fir::ResultOp>(loc, addr);
+    builder.setInsertionPointToStart(&ifOp.getElseRegion().front());
+    mlir::Value byteSize =
+        isModes ? fir::runtime::genGetModesTypeSize(builder, loc)
+                : fir::runtime::genGetStatusTypeSize(builder, loc);
+    byteSize = builder.createConvert(loc, builder.getIndexType(), byteSize);
+    addr =
+        builder.create<fir::AllocMemOp>(loc, extractSequenceType(heapTy),
+                                        /*typeparams=*/std::nullopt, byteSize);
+    mlir::Value shape = builder.create<fir::ShapeOp>(loc, byteSize);
+    builder.create<fir::StoreOp>(
+        loc, builder.create<fir::EmboxOp>(loc, fieldTy, addr, shape), fieldRef);
+    builder.create<fir::ResultOp>(loc, addr);
+    builder.setInsertionPointAfter(ifOp);
+    addr = builder.create<fir::ConvertOp>(loc, ptrTy, ifOp.getResult(0));
+  } else {
+    // Place floating point environment data in __data storage.
+    addr = builder.create<fir::ConvertOp>(loc, ptrTy, getBase(args[0]));
+  }
+  llvm::StringRef func = isModes ? (isGet ? "fegetmode" : "fesetmode")
+                                 : (isGet ? "fegetenv" : "fesetenv");
+  genRuntimeCall(func, i32Ty, addr);
 }
 
 // Check that an explicit ieee_[get|set]_rounding_mode call radix value is 2.
@@ -4529,18 +4571,6 @@ void IntrinsicLibrary::genIeeeGetRoundingMode(
   builder.create<fir::StoreOp>(loc, mode, fieldRef);
 }
 
-// IEEE_GET_STATUS, IEEE_SET_STATUS
-template <bool isGet>
-void IntrinsicLibrary::genIeeeGetOrSetStatus(
-    llvm::ArrayRef<fir::ExtendedValue> args) {
-  assert(args.size() == 1);
-  mlir::Type ptrTy = builder.getRefType(builder.getIntegerType(32));
-  mlir::Type i32Ty = builder.getIntegerType(32);
-  mlir::Value addr =
-      builder.create<fir::ConvertOp>(loc, ptrTy, getBase(args[0]));
-  genRuntimeCall(isGet ? "fegetenv" : "fesetenv", i32Ty, addr);
-}
-
 // IEEE_GET_UNDERFLOW_MODE
 void IntrinsicLibrary::genIeeeGetUnderflowMode(
     llvm::ArrayRef<fir::ExtendedValue> args) {
diff --git a/flang/lib/Optimizer/Builder/Runtime/Exceptions.cpp b/flang/lib/Optimizer/Builder/Runtime/Exceptions.cpp
index 630281fdb593d7..c545b3d00b4d7e 100644
--- a/flang/lib/Optimizer/Builder/Runtime/Exceptions.cpp
+++ b/flang/lib/Optimizer/Builder/Runtime/Exceptions.cpp
@@ -42,3 +42,17 @@ void fir::runtime::genSetUnderflowMode(fir::FirOpBuilder &builder,
       fir::runtime::getRuntimeFunc<mkRTKey(SetUnderflowMode)>(loc, builder)};
   builder.create<fir::CallOp>(loc, func, flag);
 }
+
+mlir::Value fir::runtime::genGetModesTypeSize(fir::FirOpBuilder &builder,
+                                              mlir::Location loc) {
+  mlir::func::FuncOp func{
+      fir::runtime::getRuntimeFunc<mkRTKey(GetModesTypeSize)>(loc, builder)};
+  return builder.create<fir::CallOp>(loc, func).getResult(0);
+}
+
+mlir::Value fir::runtime::genGetStatusTypeSize(fir::FirOpBuilder &builder,
+                                               mlir::Location loc) {
+  mlir::func::FuncOp func{
+      fir::runtime::getRuntimeFunc<mkRTKey(GetStatusTypeSize)>(loc, builder)};
+  return builder.create<fir::CallOp>(loc, func).getResult(0);
+}
diff --git a/flang/module/__fortran_ieee_exceptions.f90 b/flang/module/__fortran_ieee_exceptions.f90
index 6691012eda238a..3ac9b993186aab 100644
--- a/flang/module/__fortran_ieee_exceptions.f90
+++ b/flang/module/__fortran_ieee_exceptions.f90
@@ -36,13 +36,15 @@
     ieee_all(*) = [ ieee_usual, ieee_underflow, ieee_inexact ]
 
   type, public :: ieee_modes_type ! Fortran 2018, 17.7
-    private ! opaque fenv.h femode_t data
+    private ! opaque fenv.h femode_t data; code will access only one component
     integer(kind=4) :: __data(_FORTRAN_RUNTIME_IEEE_FEMODE_T_EXTENT)
+    integer(kind=1), allocatable :: __allocatable_data(:)
   end type ieee_modes_type
 
   type, public :: ieee_status_type ! Fortran 2018, 17.7
-    private ! opaque fenv.h fenv_t data
+    private ! opaque fenv.h fenv_t data; code will access only one component
     integer(kind=4) :: __data(_FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT)
+    integer(kind=1), allocatable :: __allocatable_data(:)
   end type ieee_status_type
 
 ! Define specifics with 1 LOGICAL or REAL argument for generic G.
diff --git a/flang/runtime/exceptions.cpp b/flang/runtime/exceptions.cpp
index 2fa2baa2ec84a2..9ad25dcd1dcfc1 100644
--- a/flang/runtime/exceptions.cpp
+++ b/flang/runtime/exceptions.cpp
@@ -15,8 +15,7 @@
 #include <xmmintrin.h>
 #endif
 
-// When not supported, these macro are undefined in cfenv.h,
-// set them to zero in that case.
+// fenv.h may not define exception macros.
 #ifndef FE_INVALID
 #define FE_INVALID 0
 #endif
@@ -73,14 +72,6 @@ uint32_t RTNAME(MapException)(uint32_t excepts) {
   return except_value;
 }
 
-// Verify that the size of ieee_modes_type and ieee_status_type objects from
-// intrinsic module file __fortran_ieee_exceptions.f90 are large enough to
-// hold fenv_t object.
-// TODO: fenv_t can be way larger than
-//	sizeof(int) * _FORTRAN_RUNTIME_IEEE_FENV_T_EXTENT
-// on some systems, e.g. Solaris, so omit object size comparison for now.
-// TODO: consider femode_t object size comparison once its more mature.
-
 // Check if the processor has the ability to control whether to halt or
 // continue execution when a given exception is raised.
 bool RTNAME(SupportHalting)([[maybe_unused]] uint32_t except) {
@@ -119,5 +110,16 @@ void RTNAME(SetUnderflowMode)(bool flag) {
 #endif
 }
 
+size_t RTNAME(GetModesTypeSize)(void) {
+#ifdef _AIX
+  return 8; // femode_t is not defined
+#else
+  return sizeof(femode_t); // byte size of ieee_modes_type data
+#endif
+}
+size_t RTNAME(GetStatusTypeSize)(void) {
+  return sizeof(fenv_t); // byte size of ieee_status_type data
+}
+
 } // extern "C"
 } // namespace Fortran::runtime
diff --git a/flang/test/Lower/Intrinsics/ieee_femodes.f90 b/flang/test/Lower/Intrinsics/ieee_femodes.f90
index abb264cb027eac..39976a57b8a0f8 100644
--- a/flang/test/Lower/Intrinsics/ieee_femodes.f90
+++ b/flang/test/Lower/Intrinsics/ieee_femodes.f90
@@ -1,61 +1,68 @@
-! RUN: bbc -emit-fir -o - %s | FileCheck %s
+! RUN: bbc -emit-hlfir -o - %s | FileCheck %s
+! UNSUPPORTED: sparc-target-arch
 
 ! CHECK-LABEL: c.func @_QQmain
 program m
   use ieee_arithmetic
   use ieee_exceptions
 
-  ! CHECK:  %[[VAL_69:.*]] = fir.alloca !fir.type<_QM__fortran_ieee_exceptionsTieee_modes_type{_QM__fortran_ieee_exceptionsTieee_modes_type.__data:!fir.array<2xi32>}> {bindc_name = "modes", uniq_name = "_QFEmodes"}
-  ! CHECK:  %[[VAL_70:.*]] = fir.declare %[[VAL_69]] {uniq_name = "_QFEmodes"} : (!fir.ref<!fir.type<_QM__fortran_ieee_exceptionsTieee_modes_type{_QM__fortran_ieee_exceptionsTieee_modes_type.__data:!fir.array<2xi32>}>>) -> !fir.ref<!fir.type<_QM__fortran_ieee_exceptionsTieee_modes_type{_QM__fortran_ieee_exceptionsTieee_modes_type.__data:!fir.array<2xi32>}>>
+  ! CHECK:     %[[V_59:[0-9]+]] = fir.address_of(@_QFEmodes) : !fir.ref<!fir.type<_QM__fortran_ieee_exceptionsTieee_modes_type{_QM__fortran_ieee_exceptionsTieee_modes_type.__data:!fir.array<2xi32>,_QM__fortran_ieee_exceptionsTieee_modes_type.__allocatable_data:!fir.box<!fir.heap<!fir.array<?xi8>>>}>>
+  ! CHECK:     %[[V_60:[0-9]+]]:2 = hlfir.declare %[[V_59]] {uniq_name = "_QFEmodes"} : (!fir.ref<!fir.type<_QM__fortran_ieee_exceptionsTieee_modes_type{_QM__fortran_ieee_exceptionsTieee_modes_type.__data:!fir.array<2xi32>,_QM__fortran_ieee_exceptionsTieee_modes_type.__allocatable_data:!fir.box<!fir.heap<!fir.array<?xi8>>>}>>) -> (!fir.ref<!fir.type<_QM__fortran_ieee_exceptionsTieee_modes_type{_QM__fortran_ieee_exceptionsTieee_modes_type.__data:!fir.array<2xi32>,_QM__fortran_ieee_exceptionsTieee_modes_type.__allocatable_data:!fir.box<!fir.heap<!fir.array<?xi8>>>}>>, !fir.ref<!fir.type<_QM__fortran_ieee_exceptionsTieee_modes_type{_QM__fortran_ieee_exceptionsTieee_modes_type.__data:!fir.array<2xi32>,_QM__fortran_ieee_exceptionsTieee_modes_type.__allocatable_data:!fir.box<!fir.heap<!fir.array<?xi8>>>}>>)
   type(ieee_modes_type) :: modes
 
-  ! CHECK:  %[[VAL_71:.*]] = fir.alloca !fir.type<_QM__fortran_builtinsT__builtin_ieee_round_type{_QM__fortran_builtinsT__builtin_ieee_round_type.mode:i8}> {bindc_name = "round", uniq_name = "_QFEround"}
-  ! CHECK:  %[[VAL_72:.*]] = fir.declare %[[VAL_71]] {uniq_name = "_QFEround"} : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_ieee_round_type{_QM__fortran_builtinsT__builtin_ieee_round_type.mode:i8}>>) -> !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_ieee_round_type{_QM__fortran_builtinsT__builtin_ieee_round_type.mode:i8}>>
+  ! CHECK:     %[[V_61:[0-9]+]] = fir.alloca !fir.type<_QM__fortran_builtinsT__builtin_ieee_round_type{_QM__fortran_builtinsT__builtin_ieee_round_type.mode:i8}> {bindc_name = "round", uniq_name = "_QFEround"}
+  ! CHECK:     %[[V_62:[0-9]+]]:2 = hlfir.declare %[[V_61]] {uniq_name = "_QFEround"}
   type(ieee_round_type) :: round
 
-  ! CHECK:  %[[VAL_78:.*]] = fir.address_of(@_QQro._QM__fortran_builtinsT__builtin_ieee_round_type.0) : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_ieee_round_type{_QM__fortran_builtinsT__builtin_ieee_round_type.mode:i8}>>
-  ! CHECK:  %[[VAL_79:.*]] = fir.declare %[[VAL_78]] {fortran_attrs = #fir.var_attrs<parameter>, uniq_name = "_QQro._QM__fortran_builtinsT__builtin_ieee_round_type.0"} : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_ieee_round_type{_QM__fortran_builtinsT__builtin_ieee_round_type.mode:i8}>>) -> !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_ieee_round_type{_QM__fortran_builtinsT__builtin_ieee_round_type.mode:i8}>>
-
-  ! CHECK:  %[[VAL_80:.*]] = fir.field_index _QM__fortran_builtinsT__builtin_ieee_round_type.mode, !fir.type<_QM__fortran_builtinsT__builtin_ieee_round_type{_QM__fortran_builtinsT__builtin_ieee_round_type.mode:i8}>
-  ! CHECK:  %[[VAL_81:.*]] = fir.coordinate_of %[[VAL_79]], %[[VAL_80]] : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_ieee_round_type{_QM__fortran_builtinsT__builtin_ieee_round_type.mode:i8}>>, !fir.field) -> !fir.ref<i8>
-  ! CHECK:  %[[VAL_82:.*]] = fir.load %[[VAL_81]] : !fir.ref<i8>
-  ! CHECK:  %[[VAL_83:.*]] = fir.convert %[[VAL_82]] : (i8) -> i32
-  ! CHECK:  fir.call @llvm.set.rounding(%[[VAL_83]]) fastmath<contract> : (i32) -> ()
+  ! CHECK:     %[[V_68:[0-9]+]] = fir.address_of(@_QQro._QM__fortran_builtin...
[truncated]

@github-actions
Copy link

github-actions bot commented Jan 7, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@rorth
Copy link
Collaborator

rorth commented Jan 7, 2025

This doesn't compile on Solaris:

/vol/llvm/src/llvm-project/local/flang/runtime/exceptions.cpp:114:5: error: function-like macro '__GLIBC_USE' is not defined
  114 | #if __GLIBC_USE(IEC_60559_BFP_EXT)
      |     ^

@vdonaldson
Copy link
Contributor Author

This doesn't compile on Solaris:

/vol/llvm/src/llvm-project/local/flang/runtime/exceptions.cpp:114:5: error: function-like macro '__GLIBC_USE' is not defined
  114 | #if __GLIBC_USE(IEC_60559_BFP_EXT)
      |     ^

Thanks for checking that. Is femode_t defined in /usr/include/bits/fenv.h? If yes, is it under some preprocessor directive?

@rorth
Copy link
Collaborator

rorth commented Jan 7, 2025

Thanks for checking that. Is femode_t defined in /usr/include/bits/fenv.h? If yes, is it under some preprocessor directive?

No, it isn't defined anywhere on Solaris.

@kkwli
Copy link
Collaborator

kkwli commented Jan 7, 2025

This doesn't compile on Solaris:

/vol/llvm/src/llvm-project/local/flang/runtime/exceptions.cpp:114:5: error: function-like macro '__GLIBC_USE' is not defined
  114 | #if __GLIBC_USE(IEC_60559_BFP_EXT)
      |     ^

I got the same error on AIX. And /usr/include/fenv.h on AIX does not define femode_t either.

@vdonaldson
Copy link
Contributor Author

This doesn't compile on Solaris:

/vol/llvm/src/llvm-project/local/flang/runtime/exceptions.cpp:114:5: error: function-like macro '__GLIBC_USE' is not defined
  114 | #if __GLIBC_USE(IEC_60559_BFP_EXT)
      |     ^

I got the same error on AIX. And /usr/include/fenv.h on AIX does not define femode_t either.

@rorth, @kkwli - Could you please check if this iteration of the code builds?

@kkwli
Copy link
Collaborator

kkwli commented Jan 8, 2025

This doesn't compile on Solaris:

/vol/llvm/src/llvm-project/local/flang/runtime/exceptions.cpp:114:5: error: function-like macro '__GLIBC_USE' is not defined
  114 | #if __GLIBC_USE(IEC_60559_BFP_EXT)
      |     ^

I got the same error on AIX. And /usr/include/fenv.h on AIX does not define femode_t either.

@rorth, @kkwli - Could you please check if this iteration of the code builds?

It builds successfully on AIX. Thanks.

Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please wait for the feedback from @rorth on your latest iteration. Lowering code looks good to me.

@rorth
Copy link
Collaborator

rorth commented Jan 8, 2025

While this version builds on Solaris (both sparcv9 and amd64) and shows clean test results on Solaris/amd64, SPARC is different: on both Solaris/sparcv9 (which lacks femode_t) and Linux/sparc64 (which has it) I get two test failures:

  Flang :: Lower/Intrinsics/ieee_femodes.f90
  Flang :: Lower/Intrinsics/ieee_festatus.f90

On both Solaris and LInux I get

/vol/llvm/src/llvm-project/local/flang/test/Lower/Intrinsics/ieee_femodes.f90:35:11: error: CHECK: expected string not found in input
 ! CHECK: %[[V_98:[0-9]+]] = fir.convert %[[V_60]]#1 : (!fir.ref<!fir.type<_QM__fortran_ieee_exceptionsTieee_modes_type{_QM__fortran_ieee_exceptionsTieee_modes_type.__data:!fir.array<2xi32>,_QM__fortran_ieee_exceptionsTieee_modes_type.__allocatable_data:!fir.box<!fir.heap<!fir.array<?xi8>>>}>>) -> !fir.ref<i32>
          ^
<stdin>:85:37: note: scanning from here
 fir.store %77 to %75 : !fir.ref<i8>
                                    ^
<stdin>:85:37: note: with "V_60" equal to "60" 
 fir.store %77 to %75 : !fir.ref<i8>
                                    ^

@rorth
Copy link
Collaborator

rorth commented Jan 8, 2025

I don't think the last build fix is right:

  • The two tests PASS on Solaris/amd64, which is alsosystem-solaris.
  • Likewise, the tests FAIL on Linux/sparc64, although that does have femode_t.

@vdonaldson
Copy link
Contributor Author

@kkwli - Thanks for the checking the AIX build.

@rorth - Thanks for checking solaris builds. The generated code should be different for these tests for at least some sparc environments. I've updated the test with a different UNSUPPORTED directive to address that. Could you please check if that eliminates the test failure? If not, we need to find a directive that is applicable for this scenario.

@rorth
Copy link
Collaborator

rorth commented Jan 8, 2025

As I'd already mentioned, system-solaris is wrong. Besides, if you ever consider using sparc-specific features, *-target-arch won't work: it only exists in compiler-rt/test, certainly not in flang/test.

@vdonaldson
Copy link
Contributor Author

I don't think the last build fix is right:

* The two tests `PASS` on Solaris/amd64, which is also`system-solaris`.

* Likewise, the tests `FAIL` on Linux/sparc64, although that does have `femode_t`.

This PR allows different code to be generated for sparc / solaris environments vs. other environments since the relevant data sizes are substantially different. I expect the code should be correct for the various cases, but I have no way to verify that myself. Given that, for the tests I would like to find a directive that suppresses the tests for environments that generate alternate code.

@vdonaldson
Copy link
Contributor Author

As I'd already mentioned, system-solaris is wrong. Besides, if you ever consider using sparc-specific features, *-target-arch won't work: it only exists in compiler-rt/test, certainly not in flang/test.

Ok. I'm looking for suggestions. One option is to avoid test problems by deleting the tests.

@rorth
Copy link
Collaborator

rorth commented Jan 8, 2025

This PR allows different code to be generated for sparc / solaris environments vs. other environments since the relevant data sizes are substantially different. I expect the code should be correct for the various cases, but I have no way to verify that myself. Given that, for the tests I would like to find a directive that suppresses the tests for environments that generate alternate code.

Actually, you can verify things on SPARC yourself: there's both a Solaris/sparcv9 and a Linux/sparc64 system in the Compile Farm (and many other systems on top of those).

Besides, if you can tell me exactly what to check, I could do this for both Solaris and Linux. I think that would be better than blindly XFAILing the tests without understanding the issues.

@vdonaldson
Copy link
Contributor Author

This PR allows different code to be generated for sparc / solaris environments vs. other environments since the relevant data sizes are substantially different. I expect the code should be correct for the various cases, but I have no way to verify that myself. Given that, for the tests I would like to find a directive that suppresses the tests for environments that generate alternate code.

Actually, you can verify things on SPARC yourself: there's both a Solaris/sparcv9 and a Linux/sparc64 system in the Compile Farm (and many other systems on top of those).

Besides, if you can tell me exactly what to check, I could do this for both Solaris and Linux. I think that would be better than blindly XFAILing the tests without understanding the issues.

The two tests are executable Fortran tests. Output for both tests lists expected output on the left side of each line, and actual output on the right. Is that output correct?

@vdonaldson
Copy link
Contributor Author

Thanks @rorth and @kkwli for help with resolving build issues.

The intent of this PR is to allow more flexibility in how these four floating point environment procedures are implemented, and use that flexibility to solve several problems. I believe it now does that without breaking builds or introducing regressions to working functionality in any environment. I am less concerned with lit test coverage, although the included tests address common use cases. I propose to push this iteration of the code. Anyone may extend or improve tests or code as occasion arises.

Copy link
Collaborator

@kkwli kkwli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rorth
Copy link
Collaborator

rorth commented Jan 9, 2025

I've now retested the latest version on 4 targets, but it's still not right:

  • While the patch compiles and all tests PASS on both x86_64-pc-linux-gnu and amd64-pc-solaris2.11
  • on both sparcv9-sun-solaris2.11 and sparc64-unknown-linux-gnu, I get the same failures as before:
    Flang :: Lower/Intrinsics/ieee_femodes.f90
    Flang :: Lower/Intrinsics/ieee_festatus.f90
    

This happens because the REQUIRES: x86-registered-target doesn't do what you think it does: it checks if flang is configured to support the X86 target, which is true in both my targets (configured with -DLLVM_TARGETS_TO_BUILD=all). However, you make no use whatsoever of this information: bbc is still invoked without an explicit -target option, so runs for the native/default target instead, which is sparc*-*-* in this case.

@rorth
Copy link
Collaborator

rorth commented Jan 9, 2025

Besides, if you can tell me exactly what to check, I could do this for both Solaris and Linux. I think that would be better than blindly XFAILing the tests without understanding the issues.

The two tests are executable Fortran tests. Output for both tests lists expected output on the left side of each line, and actual output on the right. Is that output correct?

If that's the case, shouldn't the tests check if they produce this expected output?

I get that output only on Linux/x86_64: I've just compiled the test with flang -o ieee_femodes.exe ieee_femodes.f90 in all cases.

  • On Linux/x86_64, it compiles, links and runs with the expected output.
  • On Solaris/amd64, the test fails to link:
    Undefined			first referenced
     symbol  			    in file
    ieee_get_modes_0_                   /var/tmp/ieee_femodes-440ad6.o
    ieee_set_modes_0_                   /var/tmp/ieee_femodes-440ad6.o
    
    Comparing the .s files between Linux and Solaris, I find that where Linux has a call to fegetmode, Solaris has one to ieee_get_modes_0_ instead: not really astonishing since Solaris lacks TS 18661-1:2014 support. It would certainly be better if flang wouldn't generate calls to functions known not to exist here, though.
  • On both Solaris/sparcv9 and Linux/sparc64, things are way worse, however: flang errors out like this:
LLVM ERROR: Cannot select: t137: ch = set_rounding t133, t253
  t253: i32,ch = load<(dereferenceable invariant load (s8) from @_QQroX_QM__fortran_builtinsT__builtin_ieee_round_typeX1, align 8), sext from i8> t0, t287, undef:i64
    t287: i64 = add t284, t286
      t284: i64 = shl t283, Constant:i32<12>
        t283: i64 = add t280, t282
          t280: i64 = SPISD::Hi TargetGlobalAddress:i64<ptr @_QQroX_QM__fortran_builtinsT__builtin_ieee_round_typeX1> 0 [TF=3]
            t279: i64 = TargetGlobalAddress<ptr @_QQroX_QM__fortran_builtinsT__builtin_ieee_round_typeX1> 0 [TF=3]
          t282: i64 = SPISD::Lo TargetGlobalAddress:i64<ptr @_QQroX_QM__fortran_builtinsT__builtin_ieee_round_typeX1> 0 [TF=4]
            t281: i64 = TargetGlobalAddress<ptr @_QQroX_QM__fortran_builtinsT__builtin_ieee_round_typeX1> 0 [TF=4]
        t274: i32 = Constant<12>
      t286: i64 = SPISD::Lo TargetGlobalAddress:i64<ptr @_QQroX_QM__fortran_builtinsT__builtin_ieee_round_typeX1> 0 [TF=5]
        t285: i64 = TargetGlobalAddress<ptr @_QQroX_QM__fortran_builtinsT__builtin_ieee_round_typeX1> 0 [TF=5]
    t1: i64 = undef
In function: _QQmain
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: tools/clang/stage2-bins/bin/flang -fc1 -triple sparcv9-sun-solaris2.11 -emit-obj -fcolor-diagnostics -mrelocation-model static -resource-dir tools/clang/stage2-bins/lib/clang/20 -mframe-pointer=all -o /var/tmp/ieee_femodes-b9df56.o -x f95-cpp-input /vol/llvm/src/llvm-project/local/flang/test/Lower/Intrinsics/ieee_femodes.f90
1.	Running pass 'Function Pass Manager' on module 'FIRModule'.
2.	Running pass 'SPARC DAG->DAG Pattern Instruction Selection' on function '@_QQmain'
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  flang-20  0x00000001084fe65c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 36
1  flang-20  0x00000001084fef60 SignalHandler(int) + 896
2  libc.so.1 0xffffffff7eec6068 __sighndlr + 12
3  libc.so.1 0xffffffff7eeb8910 call_user_handler + 1024
4  libc.so.1 0xffffffff7eeb8d00 sigacthandler + 208
5  libc.so.1 0xffffffff7eecb180 __lwp_sigqueue + 8
6  libc.so.1 0xffffffff7ede933c abort + 252
7  flang-20  0x000000010846502c llvm::report_fatal_error(llvm::Twine const&, bool) + 836
8  flang-20  0x000000010afa78b0 llvm::SelectionDAGISel::CannotYetSelect(llvm::SDNode*) + 636
9  flang-20  0x000000010afa64ec llvm::SelectionDAGISel::SelectCodeCommon(llvm::SDNode*, unsigned char const*, unsigned int) + 26512
10 flang-20  0x0000000107d22d50 (anonymous namespace)::SparcDAGToDAGISel::Select(llvm::SDNode*) + 5780
11 flang-20  0x000000010af98708 llvm::SelectionDAGISel::DoInstructionSelection() + 1756
12 flang-20  0x000000010af974b8 llvm::SelectionDAGISel::CodeGenAndEmitDAG() + 6792
13 flang-20  0x000000010af952a4 llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) + 9176
14 flang-20  0x000000010af91980 llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) + 932
15 flang-20  0x000000010af8ece8 llvm::SelectionDAGISelLegacy::runOnMachineFunction(llvm::MachineFunction&) + 216
16 flang-20  0x000000010c028f14 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) + 736
17 flang-20  0x000000010dd0c05c llvm::FPPassManager::runOnFunction(llvm::Function&) + 600
18 flang-20  0x000000010dd13d78 llvm::FPPassManager::runOnModule(llvm::Module&) + 68
19 flang-20  0x000000010dd0c800 llvm::legacy::PassManagerImpl::run(llvm::Module&) + 1192
20 flang-20  0x00000001087cc9ec Fortran::frontend::CodeGenAction::executeAction() + 2928
21 flang-20  0x00000001085584f0 Fortran::frontend::FrontendAction::execute() + 12
22 flang-20  0x0000000108543670 Fortran::frontend::CompilerInstance::executeAction(Fortran::frontend::FrontendAction&) + 356
23 flang-20  0x000000010855d114 Fortran::frontend::executeCompilerInvocation(Fortran::frontend::CompilerInstance*) + 5192
24 flang-20  0x0000000106762b6c fc1_main(llvm::ArrayRef<char const*>, char const*) + 784
25 flang-20  0x00000001067619d0 main + 5344
26 flang-20  0x0000000106760204 _start + 100
flang-20: error: unable to execute command: Abort (core dumped)
flang-20: error: flang frontend command failed due to signal (use -v to see invocation)
flang version 20.0.0git
Target: sparcv9-sun-solaris2.11
Thread model: posix
InstalledDir: tools/clang/stage2-bins/bin
Build config: +assertions

No idea if this is a SPARC backend bug or a bug in Flang.

@vdonaldson
Copy link
Contributor Author

Thank you @rorth for taking the time to build these compilers and provide detailed analysis of what happens in these environments.

These correctness problems exist without the code in this PR. I do not understand the details necessary to fully implement this functionality in environments that do not have access to the libm calls available in linux x86 environments. And I can't currently justify additional research or effort to that end. This iteration of the PR therefore reverts to generating a compile time "not yet implemented" message for calls to these procedures in environments that do not have this libm support. The code still addresses eventual cross compilation and floating point environment data size issues. It should provide a base for eventual implementation of this functionality in additional environments whenever someone chooses to do that. As it is not obvious to me how to exercise the two lit tests only in the appropriate cases I have deleted the tests. I don't think these tests are very useful. There are end-to-end tests for these calls in Fortran test suites. That should allow this code to build in all environments and either generate correct executable code or a compile time message about missing functionality.

@rorth
Copy link
Collaborator

rorth commented Jan 13, 2025

Thanks. This way, I again get clean test results on all four targets tested.

I tried reviving the last versions of ieee_femodes.f90 and ieee_festatus.f90, adding explicit -target x86_64-unknown-linux-gnu args. This way, they do PASS on both x86_64-pc-linux-gnu and sparc64-unknown-linux-gnu, but FAIL
on amd64-pc-solaris2.11 and sparcv9-sun-solaris2.11 with IntrinsicCall.cpp:4507: not yet implemented: intrinsic module procedure: ieee_get_status. This is sort of unexpected: when targeting Linux, the tests should PASS on non-Linux hosts. I believe the code in IntrinsicCall.cpp:IntrinsicLibrary::genIeeeGetOrSetModesOrStatus might be confused. Right now, by checking __GLIBC_USE_IEC_60559_BFP_EXT it checks a host property, while the existance of fe[gs]etmode/fe[gs]etenv is a property of the target system. So there may well lurk a fundamental issue here.

But as you say, it's unclear what equivalents to those TS 18661-1:2014 functions one could use (if host compiler needs to actually call them, not just the runtime), and I fully understand that you cannot justify investigating other systems.

@vdonaldson
Copy link
Contributor Author

Yes, I agree on all counts. The code takes steps toward eventually supporting cross compilation, but the temporary TODO in genIeeeGetOrSetModesOrStatus assumes host = target. The current goal for the PR is to conservatively leave the the code in a state that is correct in non cross compilation environments. The TODO may be triggered for some cases that could be handled now. This should provide an improved base for future code to build on. I'm still considering a few tweaks to the code.

Intrinsic module procedures ieee_get_modes, ieee_set_modes, ieee_get_status,
and ieee_set_status store and retrieve opaque data values whose size varies
by machine and OS environment. These data values are usually, but not always
small. Their sizes are not directly known in a cross compilation environment.
Address this by implementing two mechanisms for processing these data values.
Environments that use typical small data sizes can access storage defined
at compile time. When this is not valid, data storage of any size can be
allocated at runtime. Calls in environments that do not have access to the
underlying libm functions will generate a compile time "not yet implemented"
message.
@vdonaldson vdonaldson merged commit ff862d6 into llvm:main Jan 15, 2025
8 checks passed
@vdonaldson vdonaldson deleted the vkd1 branch January 21, 2025 21:00
static constexpr uint32_t v{FE_INVALID};
static constexpr uint32_t s{__FE_DENORM}; // subnormal
#if __x86_64__
static constexpr uint32_t s{__FE_DENORM}; // nonstandard, not a #define
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If __FE_DENORM isn't a macro and isn't standard, how can we rely on it being defined? This breaks the build on FreeBSD with the base system llvm-18 based clang compiler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:fir-hlfir flang:runtime flang:semantics flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants